Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feature/oo adaptivity2 #358

Merged
merged 76 commits into from
Nov 9, 2023
Merged

Feature/oo adaptivity2 #358

merged 76 commits into from
Nov 9, 2023

Conversation

drreynolds
Copy link
Collaborator

@drreynolds drreynolds commented Oct 26, 2023 •
edited by balos1
Loading

New object-oriented infrastructure for time step adaptivity controllers. These are currently used in ARKODE. In the not-distant future, we can implement additional step/order controllers within this existing infrastructure to support CVODE(S) and IDA(S) (and maybe even ARKODE).

Also:

drreynolds and others added 20 commits October 18, 2023 14:55
@drreynolds
Propagated SUNAdaptController class and usage from feature/oo-adaptiv…
e6bfc92 
…ity branch
@drreynolds
Added no-op SUNAdaptController implementation, to be used with fixed …
f94e99b 
…stepsize runs
@drreynolds
Moved stepsize bound enforcement inside arkCheckTemporalError, follow…
7f4c52c 
…ing David's suggestion in feature/oo-adaptivity PR review
@drreynolds
Minor update to fix a corner case arising from the previous commit
0e3433f
@drreynolds
Fixed C90 errors
d1e53f3
@drreynolds
Moved 'pq' flag out of individual controllers and back into ARKodeHAd…
47a7e88 
…aptMem, so that it would be easier to deprecate the idea later on
@drreynolds
Moved order of accuracy adjustment factor into ARKodeHAdaptMem, simpl…
b10c7bc 
…ifying SUNAdaptController API
@drreynolds
Updated lenrw and leniw in .out files
61b24fb
@drreynolds
Cleaned up documentation regarding time adaptivity controllers
7311e5c
@drreynolds
Cleaned up documentation regarding time adaptivity controllers
c63ac03
@gardner48
update workspace sizes in output files
6b30605
@drreynolds
Removed method order storage from SUNController implementations, and …
04dcf8f 
…instead pass that in as argument to EstimateStep
@drreynolds
Merge branch 'feature/oo-adaptivity2' of github.com:LLNL/sundials int…
75f1a1c 
…o feature/oo-adaptivity2
@drreynolds
Added Soderlind SUNAdaptController implementation
7b31a0d
@drreynolds
Removed PID, PI and I SUNAdaptController implementations, and instead…
e78c7ee 
… created SUNAdaptController_Soderlind utility routines for these modules
@drreynolds
Fixed CMakeLists.txt for removed SUNAdaptController modules
2fca795
@drreynolds
Fixed unused variable error
791170f
@drreynolds
Removed ExpGus and ImpGus SUNAdaptController implementations, absorbi…
833ef70 
…ng them into the Soderlind controller
@drreynolds
Updated real/integer workspace sizes in some .out files; updated test…
0d35898 
…/answers to point to corresponding PR commit
@drreynolds
Updated lenrw and leniw in some more .out files
4ac5928
@drreynolds drreynolds mentioned this pull request Oct 26, 2023
Closed
@drreynolds drreynolds added this to the SUNDIALS Next milestone Oct 26, 2023
balos1
balos1 previously approved these changes Nov 7, 2023
View reviewed changes
gardner48
gardner48 requested changes Nov 8, 2023
View reviewed changes
Copy link
Member

@gardner48 gardner48 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of minor updates to note a deprecated function. Also looks like the output files need to be updated after recent changes in develop. I'll update the output files from Jenkins in a sec.

doc/arkode/guide/source/Usage/ARKStep_c_interface/User_callable.rst Show resolved Hide resolved
src/arkode/arkode_arkstep_io.c Show resolved Hide resolved
doc/arkode/guide/source/Usage/ERKStep_c_interface/User_callable.rst Show resolved Hide resolved
src/arkode/arkode_erkstep_io.c Show resolved Hide resolved
@drreynolds
Copy link
Collaborator Author

A couple of minor updates to note a deprecated function. Also looks like the output files need to be updated after recent changes in develop. I'll update the output files from Jenkins in a sec.

I don't quite understand your final sentence -- are you planning to update the full set of answer files that are failing due to the merge in the CI, or just those that relate to the Jenkins box? Or more simply, which answer files are you not planning to update and that I'll need to handle?

@gardner48
Copy link
Member

A couple of minor updates to note a deprecated function. Also looks like the output files need to be updated after recent changes in develop. I'll update the output files from Jenkins in a sec.

I don't quite understand your final sentence -- are you planning to update the full set of answer files that are failing due to the merge in the CI, or just those that relate to the Jenkins box? Or more simply, which answer files are you not planning to update and that I'll need to handle?

I've updated the failing output files from Jenkins so that should pass on the next run of tests.

I also updated the answers submodule for GitHub CI to the latest commit in the answers repo but this will likely not resolve conflicting changes and another PR in the answers repo will be needed to update those files again.

@drreynolds
Copy link
Collaborator Author

I've updated the failing output files from Jenkins so that should pass on the next run of tests.

I also updated the answers submodule for GitHub CI to the latest commit in the answers repo but this will likely not resolve conflicting changes and another PR in the answers repo will be needed to update those files again.

Sounds good. I'll wait for the CI tests to all finish (including failures) and then update the answers repo as necessary.

@drreynolds
Copy link
Collaborator Author

I cannot log into ci/gitlab/lc.llnl.gov, so I don't know why that build/test is failing.

balos1
balos1 previously approved these changes Nov 8, 2023
View reviewed changes
@gardner48 gardner48 mentioned this pull request Nov 8, 2023
Merged
balos1 pushed a commit to sundials-codes/answers that referenced this pull request Nov 8, 2023
@gardner48 @drreynolds
Feature/oo adaptivity2 (#25)
1ff407f 
Update answers for LLNL/sundials#358

---------

Co-authored-by: Daniel R. Reynolds <[email protected]>
@gardner48 gardner48 merged commit 110a6d9 into develop Nov 9, 2023
12 of 14 checks passed
@gardner48 gardner48 deleted the feature/oo-adaptivity2 branch November 9, 2023 01:23
gardner48 added a commit that referenced this pull request Dec 18, 2023
@drreynolds @gardner48 @balos1
Feature/oo adaptivity2 (#358)
4edf370 
New object-oriented infrastructure for time step adaptivity controllers.

---------

Co-authored-by: David J. Gardner <[email protected]>
Co-authored-by: Balos, Cody, J <[email protected]>
Co-authored-by: Cody Balos <[email protected]>
balos1 added a commit that referenced this pull request Dec 18, 2023
@drreynolds @gardner48 @balos1
Feature/oo adaptivity2 (#358)
f361444 
New object-oriented infrastructure for time step adaptivity controllers.

---------

Co-authored-by: David J. Gardner <[email protected]>
Co-authored-by: Balos, Cody, J <[email protected]>
Co-authored-by: Cody Balos <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@balos1 balos1 balos1 approved these changes

@gardner48 gardner48 gardner48 approved these changes

@cswoodward cswoodward Awaiting requested review from cswoodward cswoodward is a code owner

Assignees

@balos1 balos1

@gardner48 gardner48

Labels
enhancement output-change pkg-ARKODE
Projects
None yet
Milestone
SUNDIALS v6.7.0
Development

Successfully merging this pull request may close these issues.
3 participants
@drreynolds @balos1 @gardner48